Skip to content

feat: aggregate claim-all rewards across all positions#864

Open
jinoosss wants to merge 1 commit intodevelopfrom
GSW-2608-claim-all-rewards
Open

feat: aggregate claim-all rewards across all positions#864
jinoosss wants to merge 1 commit intodevelopfrom
GSW-2608-claim-all-rewards

Conversation

@jinoosss
Copy link
Copy Markdown
Member

@jinoosss jinoosss commented Apr 23, 2026

Descriptions

Summary

Introduces a server-aggregated position rewards endpoint and wires portfolio wallet balance and claim-all flows to it. Claim-all transaction building now uses explicit LP IDs and token paths instead of passing full PositionModel[] through the repository.

Changes

  • API / repository
    • Add getPositionRewardsByAddressGET /users/{address}/position/reward.
    • Add PositionRewardsResponse DTOs (claimed / claimable groups, USD totals, positionsWithSwapFee, positionsWithStakingReward).
    • Replace ClaimAllRequest.positions with swapFeeTokenPaths, hasGnotStakingReward, positionsWithSwapFee, positionsWithStakingReward, and recipient.
  • Transactions
    • Add makeClaimAllMessageWithApprovesByIds: approvals for swap-fee tokens (pool + position), optional wugnot approval for staker when GNOT staking rewards exist, then CollectFee per LP ID and CollectReward per LP ID on the staker.
  • Data fetching
    • Add QUERY_KEY.positionRewards and useGetPositionRewards (60s refetch, invalidate on claim with positions + balances).
  • Portfolio UI
    • WalletBalanceContainer: load rewards with useGetPositionRewards, derive claim-all input and USD totals from positionRewards, keep staked / unstaked liquidity from positions (excluding closed).
    • WalletBalance / WalletBalanceDetail: consume positionRewards + tokens (path → TokenModel) for tooltips; remove client-side aggregation from converted pool positions; tooltip 1d accumulation fields are no longer populated from positions.
  • Pool detail
    • MyLiquidityContainer: build claim-all input with buildClaimAllInputFromPositions for non-closed opened positions and pass input into claimAll.
  • Hooks
    • usePosition: claimAll accepts { rpcProvider, input }; export buildClaimAllInputFromPositions and ClaimAllInput.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
gnoswap-interface Ready Ready Preview, Comment Apr 23, 2026 5:00am

Request Review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 23, 2026

Walkthrough

The PR refactors the claim-all reward flow from locally computing claimable positions to using server-derived position rewards data, introducing new query hooks, response types, and message construction logic keyed by explicit position IDs.

Changes

Cohort / File(s) Summary
Position Rewards Query Layer
packages/web/src/react-query/positions/use-get-position-rewards.ts, packages/web/src/react-query/positions/index.ts, packages/web/src/react-query/query-keys.ts
New hook useGetPositionRewards fetches position reward data via positionRepository.getPositionRewardsByAddress(), configured with 60-second polling and previous-data retention. Added query key identifier for cache management.
Position Rewards Response Types
packages/web/src/repositories/position/response/position-rewards-response.ts, packages/web/src/repositories/position/response/index.ts
New response type hierarchy (PositionRewardTokenAmount, PositionRewardsGroupResponse, PositionRewardsTotalUsd, PositionRewardsResponse) models API response structure with claimed/claimable amounts split by reward category (swap fee, internal, external).
Repository Layer
packages/web/src/repositories/position/position-repository.ts, packages/web/src/repositories/position/position-repository-impl.ts
Added getPositionRewardsByAddress() method to fetch server-side reward data. Updated sendClaimAll() to accept claim request with explicit position ID lists and token paths instead of PositionModel[], switching to makeClaimAllMessageWithApprovesByIds.
Claim-All Request Structure
packages/web/src/repositories/position/request/claim-all-request.ts
Replaced positions: PositionModel[] with four fields: swapFeeTokenPaths, hasGnotStakingReward, positionsWithSwapFee, positionsWithStakingReward to enable explicit position ID-based message construction.
Message Construction
packages/web/src/repositories/position/position.message.ts
New makeClaimAllMessageWithApprovesByIds() function builds claim messages from explicit token paths and position ID lists, replacing reward-inspection logic with caller-supplied parameters.
Position Hook Refactor
packages/web/src/hooks/pool/data/use-position.ts
Added ClaimAllInput interface and buildClaimAllInputFromPositions() helper that inspects position rewards to determine swap-fee token paths, staking reward presence, and grouped position lists. Updated claimAll flow to accept structured input instead of filtering via BigNumber.
Container Layer Refactor
packages/web/src/layouts/portfolio/containers/wallet-balance-container/WalletBalanceContainer.tsx, packages/web/src/layouts/portfolio/components/wallet-balance/WalletBalance.tsx, packages/web/src/layouts/portfolio/components/wallet-balance/wallet-balance-detail/WalletBalanceDetail.tsx
Switched from locally derived claimable amounts to server-driven positionRewards. WalletBalanceContainer fetches rewards via query hook and passes positionRewards/tokens to child components. WalletBalanceDetail now maps externally provided rewards to UI categories instead of local aggregation.
Test Updates
packages/web/src/layouts/portfolio/components/wallet-balance/WalletBalance.spec.tsx, packages/web/src/layouts/portfolio/components/wallet-balance/wallet-balance-detail/WalletBalanceDetail.spec.tsx
Added mock props positionRewards and tokens to satisfy updated component prop signatures.
My Liquidity Container
packages/web/src/layouts/pool/pool-detail/containers/my-liquidity-container/MyLiquidityContainer.tsx
Updated claimAllReward to derive ClaimAllInput from positions and pass it to claimAll instead of only passing rpcProvider.

Sequence Diagram

sequenceDiagram
    participant Component as WalletBalanceContainer
    participant Query as useGetPositionRewards
    participant Repo as PositionRepository
    participant API as /users/{address}/position/reward
    participant PositionHook as usePosition<br/>(claimAll)
    participant MsgBuilder as makeClaimAllMessageWithApprovesByIds

    Component->>Query: fetch position rewards (polling every 60s)
    Query->>Repo: getPositionRewardsByAddress(address)
    Repo->>API: GET /users/{address}/position/reward
    API-->>Repo: PositionRewardsResponse
    Repo-->>Query: PositionRewardsResponse | null
    Query-->>Component: positionRewards + positions

    Component->>Component: buildClaimAllInputFromPositions(positions)<br/>→ inspects reward categories

    Component->>PositionHook: claimAll(claimAllInput)
    PositionHook->>MsgBuilder: makeClaimAllMessageWithApprovesByIds<br/>(swapFeeTokenPaths, positionIDs)
    MsgBuilder->>MsgBuilder: build CollectFee + CollectReward<br/>messages from position IDs
    MsgBuilder->>Repo: getGRC20Allowance() for approvals
    MsgBuilder-->>PositionHook: TransactionMessage[]
    PositionHook-->>Component: transaction confirmed
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main feature: aggregating claim-all rewards across all positions through server-side endpoints and updated client-side flows.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch GSW-2608-claim-all-rewards

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
packages/web/src/layouts/pool/pool-detail/containers/my-liquidity-container/MyLiquidityContainer.tsx (2)

111-115: ⚠️ Potential issue | 🟡 Minor

Invalidate positionRewards after pool-detail claims.

Claiming changes the data returned by useGetPositionRewards, but this refresh path only invalidates balances, positions, and bins. Add the new query key so portfolio reward totals/claim-all inputs do not stay stale after claiming from pool detail.

Proposed fix
     invalidateQueryKey("MyLiquidity, Claim", [
       [QUERY_KEY.tokenBalancesByAddress, address],
       [QUERY_KEY.positions, currentChainId, address],
+      [QUERY_KEY.positionRewards, currentChainId, address],
       [QUERY_KEY.poolPairBins],
     ]);

237-251: ⚠️ Potential issue | 🟠 Major

Avoid entering loading state for an empty claim-all input.

claimAll returns null when both reward position lists are empty, but this handler already shows the pending broadcast and sets loadingTransactionClaim to true; that no-op path never resets the loading state.

Proposed fix
   const claimAllReward = () => {
-    const amount = openedPosition
-      .filter(item => !item.closed)
+    const openedClaimablePositions = openedPosition.filter(item => !item.closed);
+    const claimAllInput = buildClaimAllInputFromPositions(openedClaimablePositions);
+
+    if (claimAllInput.positionsWithSwapFee.length === 0 && claimAllInput.positionsWithStakingReward.length === 0) {
+      return;
+    }
+
+    const amount = openedClaimablePositions
       .flatMap(item => item.rewards)
       .reduce((acc, item) => acc + Number(item.claimableUsd), 0);
 
@@
 
     broadcastLoading(getMessage(DexEvent.CLAIM_FEE, "pending", messageData));
 
     setLoadingTransactionClaim(true);
-    const claimAllInput = buildClaimAllInputFromPositions(openedPosition.filter(item => !item.closed));
     claimAll({ rpcProvider, input: claimAllInput }).then(response => {
packages/web/src/layouts/portfolio/containers/wallet-balance-container/WalletBalanceContainer.tsx (1)

137-172: ⚠️ Potential issue | 🟠 Major

Reset claim loading when claimAll returns null or rejects.

Line 137 turns on the loading state, but claimAll can resolve null for empty input/missing address/caught repository errors. That path never enters the if (response) block, leaving the claim button stuck in loading.

🐛 Proposed fix
     setLoadingTransactionClaim(true);
-    claimAll({ rpcProvider, input: claimAllInput }).then(response => {
-      if (response) {
+    claimAll({ rpcProvider, input: claimAllInput })
+      .then(response => {
+        if (!response) {
+          broadcastError(BROADCAST_ERROR_VALUE.DEFAULT);
+          setLoadingTransactionClaim(false);
+          return;
+        }
+
         if (response?.code === 0 || response?.code === ERROR_VALUE.TRANSACTION_FAILED.status) {
           enqueueEvent({
             txHash: response?.data?.hash,
             action: DexEvent.CLAIM_FEE,
             checkWugnotTransfer: true,
@@
           broadcastError(BROADCAST_ERROR_VALUE.DEFAULT);
           setLoadingTransactionClaim(false);
         }
-      }
-    });
+      })
+      .catch(() => {
+        broadcastError(BROADCAST_ERROR_VALUE.DEFAULT);
+        setLoadingTransactionClaim(false);
+      });
packages/web/src/layouts/portfolio/components/wallet-balance/wallet-balance-detail/WalletBalanceDetail.tsx (1)

106-148: ⚠️ Potential issue | 🟠 Major

Don’t gate claim-all availability on tooltip token metadata.

Line 108 drops rewards when tokens lacks metadata, and Line 147 then disables claim-all based on those tooltip arrays. A missing or late token entry can hide the claim-all button even when positionRewards contains claimable position IDs.

🐛 Proposed fix
   const isClaimableAll = useMemo(() => {
     if (balanceDetailInfo.loadingPositions) return false;
-    return hasInfo(claimableRewardInfo);
-  }, [claimableRewardInfo, balanceDetailInfo.loadingPositions]);
+    return (
+      !!positionRewards &&
+      (positionRewards.positionsWithSwapFee.length > 0 || positionRewards.positionsWithStakingReward.length > 0)
+    );
+  }, [balanceDetailInfo.loadingPositions, positionRewards]);
🧹 Nitpick comments (1)
packages/web/src/hooks/pool/data/use-position.ts (1)

54-55: Remove the unused usePosition parameter instead of suppressing ESLint.

The claim-all flow now receives input at call time, so _positions is dead API surface. Please drop the parameter and update call sites such as usePosition([]) to usePosition().

♻️ Proposed refactor
-// eslint-disable-next-line `@typescript-eslint/no-unused-vars`
-export const usePosition = (_positions?: PositionModel[]) => {
+export const usePosition = () => {

As per coding guidelines, “Do not leave unused variables in code — enforced via ESLint”.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9f796952-a334-4b0a-af5c-27ad281ee384

📥 Commits

Reviewing files that changed from the base of the PR and between 573ad02 and 48c815f.

📒 Files selected for processing (16)
  • packages/web/src/hooks/pool/data/use-position.ts
  • packages/web/src/layouts/pool/pool-detail/containers/my-liquidity-container/MyLiquidityContainer.tsx
  • packages/web/src/layouts/portfolio/components/wallet-balance/WalletBalance.spec.tsx
  • packages/web/src/layouts/portfolio/components/wallet-balance/WalletBalance.tsx
  • packages/web/src/layouts/portfolio/components/wallet-balance/wallet-balance-detail/WalletBalanceDetail.spec.tsx
  • packages/web/src/layouts/portfolio/components/wallet-balance/wallet-balance-detail/WalletBalanceDetail.tsx
  • packages/web/src/layouts/portfolio/containers/wallet-balance-container/WalletBalanceContainer.tsx
  • packages/web/src/react-query/positions/index.ts
  • packages/web/src/react-query/positions/use-get-position-rewards.ts
  • packages/web/src/react-query/query-keys.ts
  • packages/web/src/repositories/position/position-repository-impl.ts
  • packages/web/src/repositories/position/position-repository.ts
  • packages/web/src/repositories/position/position.message.ts
  • packages/web/src/repositories/position/request/claim-all-request.ts
  • packages/web/src/repositories/position/response/index.ts
  • packages/web/src/repositories/position/response/position-rewards-response.ts

import { useGetAllTokenPrices } from "@query/token";
import { DexEvent } from "@repositories/common";
import { PositionConverter } from "@services/converters/position";
import { delay } from "@utils/common";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Normalize GNOT paths before deciding whether staker approval is needed.

Line 118 uses direct equality, but the position-based builder normalizes reward paths with checkGnotPath(...). If the rewards endpoint returns a native/alias GNOT path, claim-all skips the WUGNOT staker approval and the transaction can fail.

🐛 Proposed fix
-import { delay } from "@utils/common";
+import { checkGnotPath, delay } from "@utils/common";
     const hasGnotStakingReward = [
       ...positionRewards.claimable.internalReward,
       ...positionRewards.claimable.externalReward,
-    ].some(item => item.tokenPath === WRAPPED_GNOT_PATH);
+    ].some(item => checkGnotPath(item.tokenPath) === WRAPPED_GNOT_PATH);

Also applies to: 115-118

@jinoosss jinoosss self-assigned this Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant